Allow setting platform constraints with --exec_constraints #968
Conversation
coeuvre
requested review from
DaveGay,
eytankidron,
rbe-toolchains-pr-bot and
smukherj1
as code owners
coeuvre
force-pushed
the
set-exec-constraints
branch
from
f5a85a5
to
91f4d92
Compare
coeuvre
requested review from
philwo
and removed request for
smukherj1,
DaveGay,
eytankidron and
rbe-toolchains-pr-bot
coeuvre
force-pushed
the
set-exec-constraints
branch
from
ba39da9
to
ae4effc
Compare
coeuvre
force-pushed
the
set-exec-constraints
branch
from
ae4effc
to
7a3da83
Compare
|
I would be delighted to have this feature. |
coeuvre
force-pushed
the
set-exec-constraints
branch
from
7a3da83
to
8a81aa9
Compare
|
I have rebased the PR. Please review and let me know whether it's feasible to merge! |
coeuvre
force-pushed
the
set-exec-constraints
branch
from
8a81aa9
to
39f6363
Compare
|
@coeuvre I'm trying your PR and it's not overriding any of the env flags :-? When I run with I still see Also, I can't get your PR and it keeps trying to force I checked the code and, if I'm not mistaken, we still need to use Maybe there's something I'm doing wrong... :-? Anyway, it would be great if you have a working example that uses Thanks! |
|
OK, I checked further and... there's so many things hardcoded that I don't think this PR helps, sorry :-/ Here's how I got diff --git a/pkg/rbeconfigsgen/options.go b/pkg/rbeconfigsgen/options.go
index b32965a..be90748 100644
--- a/pkg/rbeconfigsgen/options.go
+++ b/pkg/rbeconfigsgen/options.go
@@ -134,12 +134,12 @@ var (
PlatformParams: PlatformToolchainsTemplateParams{
ExecConstraints: []string{
"@platforms//os:linux",
- "@platforms//cpu:x86_64",
- "@bazel_tools//tools/cpp:clang",
+ "@platforms//cpu:arm64",
+ "@bazel_tools//tools/cpp:gcc",
},
TargetConstraints: []string{
"@platforms//os:linux",
- "@platforms//cpu:x86_64",
+ "@platforms//cpu:arm64",
},
OSFamily: "Linux",
},
@@ -147,15 +147,15 @@ var (
CPPConfigRepo: "local_config_cc",
CppBazelCmd: "build",
CppGenEnv: map[string]string{
- "ABI_LIBC_VERSION": "glibc_2.19",
- "ABI_VERSION": "clang",
- "BAZEL_COMPILER": "clang",
+ "ABI_LIBC_VERSION": "glibc_2.36",
+ "ABI_VERSION": "gcc",
+ "BAZEL_COMPILER": "gcc",
"BAZEL_HOST_SYSTEM": "i686-unknown-linux-gnu",
"BAZEL_TARGET_CPU": "k8",
- "BAZEL_TARGET_LIBC": "glibc_2.19",
+ "BAZEL_TARGET_LIBC": "glibc_2.36",
"BAZEL_TARGET_SYSTEM": "x86_64-unknown-linux-gnu",
- "CC": "clang",
- "CC_TOOLCHAIN_NAME": "linux_gnu_x86",
+ "CC": "gcc",
+ "CC_TOOLCHAIN_NAME": "linux_gnu_arm64",
},
CPPToolchainTargetName: "cc-compiler-k8",
},
diff --git a/pkg/rbeconfigsgen/rbeconfigsgen.go b/pkg/rbeconfigsgen/rbeconfigsgen.go
index b7815f9..1c9799b 100644
--- a/pkg/rbeconfigsgen/rbeconfigsgen.go
+++ b/pkg/rbeconfigsgen/rbeconfigsgen.go
@@ -262,7 +262,7 @@ func workdir(os string) string {
func BazeliskDownloadInfo(os string) (string, string, error) {
switch os {
case OSLinux:
- return "https://github.com/bazelbuild/bazelisk/releases/download/v1.19.0/bazelisk-linux-amd64", "bazelisk", nil
+ return "https://github.com/bazelbuild/bazelisk/releases/download/v1.19.0/bazelisk-linux-arm64", "bazelisk", nil
case OSWindows:
return "https://github.com/bazelbuild/bazelisk/releases/download/v1.19.0/bazelisk-windows-amd64.exe", "bazelisk.exe", nil
}
I think we should have a good way to override all of the parameters, including the |
|
OK, I've managed to use your PR and avoid the hardcoding of The following works with this PR: #!/bin/bash
build() {
go build \
-o rbe_configs_gen \
./cmd/rbe_configs_gen/rbe_configs_gen.go
}
generate() {
local docker_image="$1"
local bazel_version="$2"
local target_arch="$3"
local repo_path="$4"
local exec_constraints=(
"@bazel_tools//platforms:linux"
"@bazel_tools//platforms:$target_arch"
"@bazel_tools//tools/cpp:gcc"
)
GLIBC_VERSION="2.36" # ldd --version
cat <<- EOF >/tmp/cpp_env.json
{
"ABI_LIBC_VERSION": "glibc_$GLIBC_VERSION",
"ABI_VERSION": "gcc",
"BAZEL_COMPILER": "gcc",
"BAZEL_HOST_SYSTEM": "i686-unknown-linux-gnu",
"BAZEL_TARGET_CPU": "k8",
"BAZEL_TARGET_LIBC": "glibc_$GLIBC_VERSION",
"BAZEL_TARGET_SYSTEM": "${target_arch}-unknown-linux-gnu",
"CC": "gcc",
"CC_TOOLCHAIN_NAME": "linux_gnu_$target_arch"
}
EOF
# NOTE: bazel_path is needed to avoid downloading Bazelisk inside the
# image because the tool HARDCODES THE ARCH to x86 regardless of the
# docker platform we use...
IFS=','
./rbe_configs_gen \
--generate_cpp_configs=true \
--generate_java_configs=false \
--exec_os="linux" \
--target_os="linux" \
--toolchain_container="$docker_image" \
--docker_platform="linux/$target_arch" \
--bazel_version="$bazel_version" \
--bazel_path="/usr/bin/bazel" \
--exec_constraints="${exec_constraints[*]}" \
--cpp_env_json=/tmp/cpp_env.json \
--output_src_root="$repo_path"
}
"$@"Then, using the script, I (1) build the tool locally (in my case, MacOS arm64) and (2) run $ ./runner.sh build
$ ./runner.sh generate MY_CUSTOM_IMAGE 7.3.2 arm64 /tmp/repo |
|
@coeuvre Also, it would be awesome if you could rebase your branch, I've tried and it rebases cleanly on top of the current |
|
I've also noticed that |
By default,
clangis detected for cpp configs. While--cpp_env_jsoncan be used to override the default environment variables during toolchain generation, there isn't a way to override platform constraints.This PR adds flag
--exec_constraintswhich accept a comma separated list of constraint values e.g.--exec_constraints=@bazel_tools//platforms:linux,@bazel_tools//platforms:x86_64,@bazel_tools//tools/cpp:gccto override the default values.